[ti_abusech] Add Commercial API support for MalwareBazaar data streams#20199
[ti_abusech] Add Commercial API support for MalwareBazaar data streams#20199moxarth-rathod wants to merge 4 commits into
Conversation
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
Elastic Docs Style Checker (Vale)Summary: 4 suggestions found 💡 Suggestions (4): Optional style improvements. Apply when helpful.
The Vale linter checks documentation changes against the Elastic Docs style guide. To use Vale locally or report issues, refer to Elastic style guide for Vale. |
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
ja3_fingerprints |
22222.22 | 9680.54 | -12541.68 (-56.44%) | 💔 |
malware |
13888.89 | 6090.13 | -7798.76 (-56.15%) | 💔 |
malwarebazaar |
9708.74 | 7530.12 | -2178.62 (-22.44%) | 💔 |
To see the full report comment with /test benchmark fullreport
| event.ingested: ".*" | ||
| abusech.malwarebazaar.deleted_at: ".*" |
There was a problem hiding this comment.
There is no sample event from the system tests. Can you add this?
| { | ||
| "code": 200, | ||
| "token": "test-commercial-token", | ||
| "expires": 4102444800 |
There was a problem hiding this comment.
January 1, 2100 for never expires? Has this been tested against a real endpoint so that we know the token requests work?
Alternatively, add a script test for this.
There was a problem hiding this comment.
Yes, this data stream was tested on the live instance.
Co-authored-by: Dan Kortschak <dan.kortschak@elastic.co>
| }, | ||
| "want_more": false, | ||
| } | ||
| : has(state.?token.access_token) && has(state.?token.expires) && now() < timestamp(int(state.token.expires) - 60) ? |
There was a problem hiding this comment.
Severity: 🟠 High confidence: high path: packages/ti_abusech/data_stream/malwarebazaar/agent/stream/cel.yml.hbs:48
The rendered CEL policy no longer matches the committed test/policy/*.expected files, so elastic-package test policy will fail; regenerate the expected policies from the updated template.
Details
The program: block in cel.yml.hbs contains no Handlebars variables, so the CEL rendered into the policy must be byte-for-byte identical to the program: value stored in _dev/test/policy/test-default.expected and test-all.expected. The template was refactored but the expected policy files were not regenerated, so they now diverge in several places, including:
- Template
has(state.?token.access_token) && has(state.?token.expires)vs expectedhas(state.token) && has(state.token.access_token) && has(state.token.expires). - Template
has(state.?cursor.after)/has(state.?cursor.first_seen)vs expectedhas(state.cursor) && has(state.cursor.after)/has(state.cursor) && has(state.cursor.first_seen). - Template
has(body.?meta.next_cursor)vs expectedhas(body.meta) && has(body.meta.next_cursor). - Template
"id": auth_resp.Status/"id": resp.Statusandauth_resp.Status + " ("vs expected"id": string(auth_resp.Status)andstring(auth_resp.Status) + " (".
Becauseelastic-package test policycompares the rendered template against these committed expected files, the mismatch makes the policy test fail in CI.
Recommendation:
Regenerate the policy test expected files against the current template so the stored program: matches what the template renders:
cd packages/ti_abusech
elastic-package test policy -gThen re-commit the updated _dev/test/policy/test-default.expected and _dev/test/policy/test-all.expected.
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
| required: true | ||
| show_user: true | ||
| default: 24h | ||
| description: How far back to pull malware samples from MalwareBazaar when using the Commercial API. Supported units for this parameter are d/h/m/s. Example `120h`. Default value is `24h`. |
There was a problem hiding this comment.
Severity: 🟡 Medium confidence: medium path: packages/ti_abusech/data_stream/malwarebazaar/manifest.yml:75
The initial_interval description advertises a d (days) unit, but its value is parsed by CEL duration() which rejects d; drop d from the supported units so users are not led into a runtime error.
Details
initial_interval is consumed by the CEL program as duration(state.initial_interval). mito's duration() uses Go's time.ParseDuration, which only accepts the units ns, us, ms, s, m and h — it does not support d (days). The manifest description states "Supported units for this parameter are d/h/m/s", so a user who follows the documentation and enters a value such as 30d will cause the CEL program to fail at evaluation time with an unknown-unit error. The default (24h) and the example (120h) are valid, but the advertised d unit is not.
Recommendation:
Remove the unsupported d unit from the description so it reflects what duration() actually accepts:
- name: initial_interval
type: text
title: Initial Interval
multi: false
required: true
show_user: true
default: 24h
description: How far back to pull malware samples from MalwareBazaar when using the Commercial API. Supported units for this parameter are h/m/s. Example `120h`. Default value is `24h`.🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
⚠️ Automated review — verify suggestions before applying.
|
✅ All changelog entries have the correct PR link. |
|
No issues across the latest commits e7adae0. Review summaryIssues found across earlier commits 62225c5 — 1 high, 1 medium
🤖 AI-Generated Review | Vera Review Bot | 📚 Knowledge base: integration-skills
|
💚 Build Succeeded
History
|
Proposed commit message
Checklist
changelog.ymlfile.How to test this PR locally
Related issues